Fix/#67 #1004
Fix/#67 #1004
Conversation
There was a problem hiding this comment.
Thanks for the PR!
Generally this seem okay. I added some comments, please take a look.
Also, I'm not quite sold on the showYearPickerFirst prop name. I'm thinking maybe startOnYearSelection, or initialSelection 'year' | 'date'... What would you suggest?
Also, tests need to pass. Maybe just re-running them will help.
Thank you!
| activity?.let { lifecycleOwner -> | ||
| datePicker!!.viewLifecycleOwnerLiveData.observe(lifecycleOwner) { owner -> | ||
| if (owner != null) { | ||
| datePicker?.requireDialog()?.window?.decorView?.post { | ||
| val root = datePicker!!.dialog?.window?.decorView ?: return@post | ||
|
|
||
| val yearText = initialDate.year().toString() | ||
| val hit = findViewBy(root) { v -> | ||
| v is TextView && v.isShown && v.isClickable && v.text?.toString()?.contains(yearText) == true | ||
| } | ||
| if (hit != null) { | ||
| hit.performClick() | ||
| return@post | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
this seems rather hacky... but okay. Let's say we use this.
But before we do, please
- no force unwrap in this code
- change
if (owner != null)toif (owner == null)and an early return - we're adding an observer. Should we clean it up also?
| DatePicker datePicker = datePickerDialog.getDatePicker(); | ||
|
|
||
| int yearId = Resources.getSystem().getIdentifier("date_picker_header_year", "id", "android"); | ||
| View yearView = datePicker.findViewById(yearId); |
There was a problem hiding this comment.
let's please add an early return if yearId == 0. It probably isn't needed but let's be on the safer side of things. Android is a mess so you never know.
| boolean showYearPickerFirst = args.getBoolean(RNConstants.ARG_SHOW_YEAR_PICKER_FIRST); | ||
| dialog.setOnShowListener( | ||
| combine( | ||
| openYearDialog(dialog, canOpenYearDialog, showYearPickerFirst), |
There was a problem hiding this comment.
| openYearDialog(dialog, canOpenYearDialog, showYearPickerFirst), | |
| openYearDialog(dialog, canOpenYearDialog && showYearPickerFirst), |
Fewer booleans is better than more
- Remove no force unwrap - Add a guard clause - Add observer cleanup react-native-datetimepicker#1004 (comment)
|
@vonovak I understand you’re not quite sold on the showYearPickerFirst prop name — could you clarify a bit more about what specifically doesn’t feel right with it? |
Summary
This pull request addresses issue #67 by adding a feature that allows the year picker to be shown first.
Implementation details:
Test Plan
Tested using an Android emulator.
pixcel6-android12-default_WStd3B14.mp4
pixcel6-android12-material_X3kctEEi.mp4
pixcel7-android14-default_D6ZmfYhW.mp4
pixcel7-android14-material_qUr9dRH2.mp4
nexus5x-android7-default_bIXOxaYC.mp4
nexus5x-android7-material_W1Lry5gS.mp4
What's required for testing (prerequisites)?
What are the steps to reproduce (after prerequisites)?
Compatibility
Checklist
README.mdexample/App.js)